Allow for specifying the :writable and :on_writable_violation option for :inserted_at in the timestamps macro#4752
Conversation
|
Can you please expand on which scenarios this would trigger? Which code? |
Sure thing, thanks for taking a look! I have to admit that it is a bit difficult to come up with "realistic" examples as almost any example I can think of is an obvious bug, primarily casting The one scenario I have personally seen this in production code is in a scenario similar to the following, when "overlaying" fields of a record onto another (and im certainly not saying this is good code! 🤣 ) defmodule TimestampsWritableSample do
use Ecto.Schema
schema "timestamps" do
field :foo, :integer
field :bar, :string
field :sensitive, :string
timestamps(type: :utc_datetime_usec)
end
def overlay_changeset(%__MODULE__{} = self, %__MODULE__{} = other) do
changes =
other
|> Map.from_struct()
# oops, forgot to include inserted_at and updated_at because they are behind
# a macro
|> Map.take(__MODULE__.__schema__(:fields) -- [:id, :sensitive])
Ecto.Changeset.change(self, changes)
end
end
test "overlaying a record onto another" do
a = TestRepo.insert!(%TimestampsWritableSample{id: 1, foo: 10, bar: "test1", sensitive: "sensitive1"})
b = TestRepo.insert!(%TimestampsWritableSample{id: 2, foo: 20, bar: "test2", sensitive: "sensitive2"})
updated_a =
a
|> TimestampsWritableSample.overlay_changeset(b)
|> TestRepo.update!()
refute updated_a.id == b.id
assert updated_a.foo == b.foo
assert updated_a.bar == b.bar
refute updated_a.sensitive == b.sensitive
# accidentally overwrite inserted_at :(
refute DateTime.compare(updated_a.inserted_at, b.inserted_at) == :eq
endIn my experience |
|
So I think in this case the warning is doing its job and its warning you that you are passing it when you should not. You should fix the code instead! |
@josevalim I am a bit confused, can you clarify what warning you are referring to? Today |
|
Oh, I see! It is not detected and you want to detect it. I think it should be opt-in, unfortunately, as to not break existing code. You can use @timestamp_opts or something to change the default behaviour, yeah. |
Perfect, I will update this PR to make it opt-in this evening! Thanks as always Jose! |
…for :inserted_at in the timestamps macro
f1a1ed1 to
be2a382
Compare
|
@josevalim this is ready for another look when you get a chance, no rush 👍 |
This is a follow-up to #4734.
When actually configuring
on_writable_violationin my app, I noticed that it wasn't possible to set oninserted_atdue to it being behind thetimestamps()macro. This seems like a textbook case forwritable: :insert, which this PR facilitates.